Skip to content

Update tests to be compatible with new OpenAI, MistralAI and MCP versions #2094

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

medaminezghal
Copy link
Contributor

@medaminezghal medaminezghal commented Jun 28, 2025

I've updated known model list with new OpenAI version, fixed MistralAI test issue in new version, added new ResourceLink MCP type and fixed tests for new MCP version.

Copy link
Contributor

hyperlint-ai bot commented Jun 28, 2025

PR Change Summary

Updated tests to ensure compatibility with the latest versions of OpenAI, MistralAI, and MCP.

  • Updated known model list with the new OpenAI version
  • Fixed MistralAI test issue in the new version
  • Adjusted tests for compatibility with the new MCP version

Modified Files

  • docs/mcp/server.md

How can I customize these reviews?

Check out the Hyperlint AI Reviewer docs for more information on how to customize the review.

If you just want to ignore it on this PR, you can add the hyperlint-ignore label to the PR. Future changes won't trigger a Hyperlint review.

Note specifically for link checks, we only check the first 30 links in a file and we cache the results for several hours (for instance, if you just added a page, you might experience this). Our recommendation is to add hyperlint-ignore to the PR to ignore the link check for this PR.

Copy link
Contributor

@DouweM DouweM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@medaminezghal Thanks Mohamed! The only thing I'm not sure about is how we should handle ResourceLink.

@@ -239,6 +239,13 @@ def _map_tool_result_part(
)
else:
assert_never(resource)
elif isinstance(part, mcp_types.ResourceLink):
return {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the expectation that this resource link is sent straight to the model as JSON, or are we supposed to handle it somehow and pass the actual resource?

If it's supposed to be JSON, we can use part.model_dump() here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DouweM I'm not sure either. So I have open an question in Github about the new ResourceLink and that's what I get.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DouweM Since there is no support to resource subscribing, so it's better to make it return the content of the resource?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@medaminezghal Yeah that's what I'm thinking. I was gonna wait for confirmation but I expect that's the way we'll go, so if you feel like adding it please do!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DouweM

elif isinstance(part, mcp_types.ResourceLink):
    filepath = part.uri[7:]
    with open(filepath, "rb") as f:
        file_data = f.read()
   return messages.BinaryContent(data=base64.b64encode(file_data), media_type=part.mimeType)

What about this solution?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@medaminezghal That'll only work if the resource is on the local filesystem. I think we'll need to use the resources/read endpoint: https://modelcontextprotocol.io/specification/2025-06-18/server/resources#reading-resources. In the Python SDK, that's the await session.read_resource call from this example: https://github.com/modelcontextprotocol/python-sdk#writing-mcp-clients.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DouweM What about this solution?

Do you have any suggestion?

@DouweM DouweM self-assigned this Jun 30, 2025
@DouweM
Copy link
Contributor

DouweM commented Jul 1, 2025

@medaminezghal Can you please have a look at the failing coverage check?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants